Skip to content

fix: skip PASS-verdict reviews in roborev fix#382

Merged
wesm merged 3 commits intomainfrom
fix-pass-verdict
Feb 26, 2026
Merged

fix: skip PASS-verdict reviews in roborev fix#382
wesm merged 3 commits intomainfrom
fix-pass-verdict

Conversation

@wesm
Copy link
Collaborator

@wesm wesm commented Feb 26, 2026

Summary

  • roborev fix now skips reviews with a PASS verdict instead of wastefully invoking an agent on them, matching the existing behavior in roborev refine
  • Both fixSingleJob and runFixBatch check the verdict (stored or parsed from review output) and auto-mark passing jobs as addressed
  • Adds jobVerdict helper that uses job.Verdict when populated, falling back to storage.ParseVerdict(review.Output)

Closes #372

Test plan

  • TestJobVerdict — table-driven: stored P/F, nil fallback, empty fallback
  • TestFixSingleJobSkipsPassVerdict — PASS review returns nil without agent invocation, job marked addressed
  • TestFixBatchSkipsPassVerdict — mixed PASS/FAIL batch: only FAIL job reaches agent, PASS job marked addressed (asserted by recorded job_id)
  • go test -race ./cmd/roborev -count=1 — all existing fix tests pass

🤖 Generated with Claude Code

wesm and others added 2 commits February 26, 2026 13:51
Reviews that passed (verdict "P") have no findings to fix. Both
fixSingleJob and runFixBatch now check the verdict before invoking
an agent, matching the behavior already present in refine's
findFailedReviewForBranch. Passing reviews are auto-marked as
addressed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…IDs in test

Address review feedback: the batch PASS-skip path silently discarded
markJobAddressed errors, inconsistent with fixSingleJob and the later
batch marking logic. Also strengthen TestFixBatchSkipsPassVerdict to
assert that job 10 is actually marked addressed by recording posted
job_id values.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 26, 2026

roborev: Combined Review (2748f5f)

Verdict: Clean

All code review agents agree that the code is clean and no issues were found.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

The test-pass-skip agent registered in TestFixSingleJobSkipsPassVerdict
leaked into TestSelectRefineAgentCodexFallback, causing it to find an
agent when it expects none. Add t.Cleanup to unregister after the test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@roborev-ci
Copy link

roborev-ci bot commented Feb 26, 2026

roborev: Combined Review (ba197df)

Verdict: Clean

All agents agree the code is clean and no issues were found across security and default reviews. The changes successfully implement the logic to skip and mark "PASS" jobs as addressed during fix operations with appropriate
test coverage.


Synthesized from 4 reviews (agents: codex, gemini | types: default, security)

@wesm wesm merged commit 05b27f8 into main Feb 26, 2026
8 checks passed
@wesm wesm deleted the fix-pass-verdict branch February 26, 2026 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

roborev fix may try to fix reviews with Pass verdict

1 participant